- 
                Notifications
    You must be signed in to change notification settings 
- Fork 13.9k
          Remove unstable cfg target(...) compact feature
          #130780
        
          New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
| Wasn’t this helping to remove some use cases for build scripts? | 
| The part that was supposed to help with build script was the  | 
| IIRC the target abi was only matchable in build scripts, is that still the case today (with and without this PR)? To be able to lower the need for build scripts in the ecosystem we need cfgs to be at least isomorphic… | 
| This PR doesn't change anything for matching on specific target cfgs components. This PR only removes the shorthand  As for the target abi, the  | 
| Good. Thank you for the clarifications. | 
      
        
              This comment was marked as resolved.
        
        
      
    
  This comment was marked as resolved.
79578af    to
    9832308      
    Compare
  
    
      
        
              This comment was marked as resolved.
        
        
      
    
  This comment was marked as resolved.
9832308    to
    1de1d84      
    Compare
  
    
      
        
              This comment was marked as resolved.
        
        
      
    
  This comment was marked as resolved.
1de1d84    to
    1f2c377      
    Compare
  
    
      
        
              This comment was marked as resolved.
        
        
      
    
  This comment was marked as resolved.
1f2c377    to
    7c50b97      
    Compare
  
    
      
        
              This comment was marked as resolved.
        
        
      
    
  This comment was marked as resolved.
7c50b97    to
    2cdde6a      
    Compare
  
    
      
        
              This comment was marked as resolved.
        
        
      
    
  This comment was marked as resolved.
2cdde6a    to
    c6b25b3      
    Compare
  
    
      
        
              This comment was marked as resolved.
        
        
      
    
  This comment was marked as resolved.
c6b25b3    to
    f047624      
    Compare
  
    
      
        
              This comment has been minimized.
        
        
      
    
  This comment has been minimized.
| I think we should close this for now. If another RFC comes along that obviates the need for this feature, we can remove it then. @rfcbot reviewed | 
| It's not obvious to me that is better enough than to be worth having two ways. So I think I'm weakly inclined to agree with Urgau here and just remove the short-hand. Pondering: would  | 
| Conceivably, we could have rustfmt normalize to the nicer form (after maybe considering your MSRV). In the long term, that would make it feel less like we had two ways to do it. Anyway, that's not a proposal; it's just to say we have options. | 
| I don't see how the "two ways to do it" feeling will go away entirely. For example, here's two  
 | 
| It's longer, but in certain contexts, I could imagine preferring #[cfg(any(
  target(arch="aarch64", endian="little"),
  target(arch="arm64ec", endian="little"),
))]to the more compressed form. I wonder to what degree preferring the more compressed form might just be an artifact of #[cfg(any(
  all(target_arch="aarch64", target_endian="little"),
  all(target_arch="arm64ec", target_endian="little"),
))]being even more expanded. Turning back to the compressed form, and to whether we could ever normalize to the new way, these seem about the same to me: #[cfg(all(any(target_arch="aarch64", target_arch="arm64ec"), target_endian="little"))]
#[cfg(all(any(target(arch="aarch64"), target(arch="arm64ec")), target(endian="little")))] | 
| The reason why I think replicating the  I originally had a tangent about  | 
| Yes, it depends what you're trying to emphasize, in context. | 
| FWIW, I think rust-lang/rfcs#3796 is a better solution for compactly expressing things like this. (No objection to either stabilizing or closing. Mild preference for closing.) EDIT(workingjubilee): in meeting "preference for closing" was clarified as "preference for ripping out"... the "double-negative" of this FCP got confusing | 
      
        
              This comment was marked as resolved.
        
        
      
    
  This comment was marked as resolved.
70fadcd    to
    292881e      
    Compare
  
    | ☔ The latest upstream changes (presumably #143460) made this pull request unmergeable. Please resolve the merge conflicts. | 
| By way of attempting to unblock this (in either direction): This shorthand seems like it entirely rests on the duplication of the various  We don't necessarily want that for every  In the context of  | 
| I don't really see aliases as an alternative for this.  It's appealing, I think, to have  That said, the  @rfcbot cancel | 
| @traviscross proposal cancelled. | 
| @rfcbot fcp merge | 
| Team member @traviscross has proposed to merge this. The next step is review by the rest of the tagged team members: No concerns currently listed. Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns. | 
This PR removes the unstable
cfg_target_compactfeature from #96901, which permits compacttarget(...)cfgs (e.g.cfg(target(os = "linux"))->cfg(target_os = "linux")).The feature:
target_*cfgsas it will only save you the "target_" part on the second cfg
target(os = "linux", env = "")meanstarget_os = "linux", target_env = ""in every other placeAll this to say that I don't think the feature pulls its weight.
@rustbot label +I-lang-nominated
Closes #96901